fix: block service worker registrations for blocked URLs (upstream #15135)#3490
Merged
Conversation
…RLs (upstream #15135) Configures network conditions on the service worker session before releasing it from the debugger pause, so the main script fetch respects the block/allow list. Without this, service workers registered from a blocked URL would succeed because the network conditions were never applied to the SW session. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…l.json These tests have no entry in upstream Puppeteer's TestExpectations.json or CanaryTestExpectations.json, so they don't belong in upstream.json. Moved to local.json with a note that the feature requires Chrome >= 149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Resolved conflicts in: - ChromeTargetManager.cs: kept master's version. The blocked-service-worker handling and the two-arg MaybeSetupNetworkBlockListAsync signature already landed via #3491 (upstream #15136), which superseded this branch's production change. Net result is identical to master. - NetworkRestrictionsTests.cs: kept both this branch's service-worker registration tests and master's "fetch from within a service worker" test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The two SW-registration tests were marked FAIL in local expectations "until PuppeteerSharp upgrades to Chrome 149". We're now on Chrome 150, and in this framework a FAIL expectation skips the test entirely — so these two tests were running nowhere. Verified locally on Chrome 150 (CDP, headless): both now run and pass (register() throws as expected). Removing the entries lets them actually validate the feature. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The blocked-service-worker path set up the network block and resumed the worker (Runtime.runIfWaitingForDebugger) in parallel, and enabled the Network domain in parallel with the emulate rules. Over the pipe transport the resume won the race, so the worker fetched its own (blocked) script before the rules applied and registration succeeded — the two ShouldFailServiceWorkerRegistration* tests failed there while passing over websocket. Sequence the commands deterministically: Network.enable, then the emulate rules, then resume. Now the block is guaranteed in place before the worker runs, regardless of transport. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ux+cdp+pipe Sequencing the network block before resuming the worker (the previous commit) made the blocked service worker's own script fetch hang under offline emulation — it regressed the previously-passing websocket runs to a 3-minute timeout, on Linux too (not just locally). Reverted. The underlying behavior is a genuine Chrome/transport limitation: the CDP network block does not reliably block the SW script fetch over --remote-debugging-pipe on Linux (works over websocket everywhere, and over pipe on Windows). Rather than ignore the tests everywhere (the original blanket expectation) or weaken them, scope a FAIL expectation to exactly linux+cdp+pipe so the tests still run and validate the feature in every other configuration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e on all transports) The offline-emulation block (Network.emulateNetworkConditionsByRule) for a disallowed service worker is non-deterministic: it only sets a connectivity state that races the worker's own script fetch. Depending on transport and mode it errors fast (headless+websocket → register rejects, test passes), lands too late (Linux headful/pipe → register succeeds, test fails), or stalls the fetch entirely (register hangs). And it can't be applied before the worker resumes, because a worker paused on waitForDebuggerOnStart answers no CDP command until resumed — awaiting any setup first deadlocks. Block at the Fetch layer instead: enable Fetch interception (queued ahead of runIfWaitingForDebugger on the same session, so it's active before the worker fetches anything) and fail every paused request with BlockedByClient. Fetch holds each request until we fail it, so there's no race and no stall — the registration is rejected deterministically regardless of transport. Removes the need for the local cdp-pipe expectations. Verified locally (headless, websocket): both SW-registration tests pass in ~1s (previously hung), full NetworkRestrictions class 17 passed / 6 skipped / 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports upstream #15135 — service workers registered from a blocked URL should fail to register, because the network block/allow list is applied to the service worker before it's released from the debugger pause.
The production block already landed via #3491 (upstream #15136), so this PR adds the test coverage: two tests for service-worker registration being blocked (blocklist + allowlist).
Getting those green surfaced a real reliability bug in how a disallowed service worker is blocked. Upstream uses
Network.emulateNetworkConditionsByRule {offline}, but that only sets a connectivity state that races the worker's own script fetch — depending on transport/mode it errors fast (headless+websocket → pass), lands too late (Linux headful/pipe → register succeeds → fail), or stalls the fetch and hangs. It also can't be applied before resuming the worker: a worker paused onwaitForDebuggerOnStartanswers no CDP command until resumed, so awaiting any setup first deadlocks.The fix blocks at the Fetch layer: enable
Fetchinterception (queued ahead ofrunIfWaitingForDebuggeron the same session, so it's active before the worker fetches anything) and fail every paused request withBlockedByClient. Fetch holds each request until we fail it — no race, no stall — so the registration is rejected deterministically on every transport. This diverges from upstream's offline approach (upstream only exercises headless+websocket in CI, so it never hits the race), noted in a code comment.Verified locally (headless, websocket): both tests pass in ~1s (previously hung); full NetworkRestrictions class 17 passed / 6 skipped / 0 failed. No local test expectations needed.
Upstream: puppeteer/puppeteer#15135